Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileAPI for file and image access #40

Closed
wants to merge 3 commits into from

Conversation

lucasbrendel
Copy link

@lucasbrendel lucasbrendel commented Nov 14, 2020

Adding base API class for accessings files hosted on FarmOS.

I did not add any helping methods that would allow for easy image downloading. I just wanted to mimic the other API classes to allow for getting images. I have only tested getting by id so far.

I am also not sure what you would prefer for a test for this type of a class, uploading and hosting an image and then downloading. I am open to adding any tests you feel are necessary. I have tested this with my extensions project at applecreekacres/farmer where I get the image information and then download and save it. At this time of this writing i have not pushed that to the repo.

@paul121
Copy link
Member

paul121 commented Nov 14, 2020

This is a good start @lucasbrendel! I hadn't tried working with files over the API, but looks like it might be fairly easy.

I just quickly tested the file.get() file.send() and file.delete() methods and only file.get() is working. It seems like the server only accepts GET requests on the /files/ID endpoint. This might be a limitation of the restws or restws_file modules?

I also think we have GET permissions to this endpoint disabled by default (it only worked with my Drupal admin user) which might be something worth documenting.

That said... I think farmOS 2.x will have even better support for files over the API, so I'm not opposed to including limited support for files right now, even if it is only GET.

@lucasbrendel
Copy link
Author

Thanks @paul121 for looking at the other requests. I am completely fine if this doesn't get merged because of the future with 2.x. when I was investigating getting file data it was so simple and few lines of code, just for the GET I figured I might as well push it back. All I really wanted was to download files so my needs are met for now.

@paul121
Copy link
Member

paul121 commented Feb 2, 2021

Hey @lucasbrendel ! Glad you got that working for your needs and appreciate you sharing the fix back with us :D

I'm going to go ahead and close this, it will be easier to support files in 2.x.

I'm curious for your thoughts on how we could implement this though! I left some notes here about how files work with JSONAPI: #41 (comment)

A little tutorial for reference too: https://interactiveknowledge.com/insights/how-retrieve-image-using-decoupled-jsonapi

It seems like the request to actually GET a file won't be as structured as it was in 1.x. Rather than /files/id, it might be something like /sites/default/files/2019-10/yellow-flowers.jpg. Maybe a farmOS.py method like client.file.get(id) could accept a File ID & automatically GET the URL that is returned? 🤔

@paul121 paul121 closed this Feb 2, 2021
@lucasbrendel
Copy link
Author

@paul121 i am fine with whatever you feel is necessary. The only things i care about being able to do are:

  • Get files attached to a log
  • Get files in the system (not near as important on its own for me)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants